fix: kill ghost agent processes with two-stage Ctrl+C#38
Merged
Conversation
Add _kill_process_group() and _SESSION_KWARGS to support killing agent processes and all their children when the ralph loop is cancelled or times out. This is the foundation for fixing #26. Co-authored-by: Ralphify <noreply@ralphify.co>
Use start_new_session=True in _run_agent_streaming so the agent and its children form a separate process group. Replace proc.kill() calls with _kill_process_group() for proper group-wide cleanup on timeout and in the finally block. Also harden _kill_process_group with a pgid==pid guard and SIGTERM-before-SIGKILL strategy for safer WSL behavior. Co-authored-by: Ralphify <noreply@ralphify.co>
Use subprocess.Popen with start_new_session=True in the blocking path so agent subprocesses form their own process group. On timeout or KeyboardInterrupt, the entire group is killed via _kill_process_group, preventing ghost processes from surviving Ctrl+C. Co-authored-by: Ralphify <noreply@ralphify.co>
Cover _kill_process_group behavior (SIGTERM/SIGKILL escalation, session leader guard, fallbacks) and verify both streaming and blocking modes use start_new_session and call _kill_process_group on timeout/interrupt. Co-authored-by: Ralphify <noreply@ralphify.co>
Remove redundant _kill_process_group integration tests that duplicated unit test coverage, merge similar fallback tests, and trim helper docstrings. Reduces test additions by ~100 lines without losing coverage. Co-authored-by: Ralphify <noreply@ralphify.co>
Co-authored-by: Ralphify <noreply@ralphify.co>
Merge TestKillProcessGroup and TestProcessGroupIsolation into a single TestProcessGroupCleanup class with a class-level pytestmark for the win32 skipif. Remove redundant docstrings and unused os import. Co-authored-by: Ralphify <noreply@ralphify.co>
First Ctrl+C finishes the current iteration naturally, avoiding interrupted writes. Second Ctrl+C force-kills the agent process group (SIGTERM with 3s grace, then SIGKILL). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #26. Supersedes #27.
_kill_process_grouphelper that sends SIGTERM with 3s grace period before SIGKILLstart_new_session=Trueto isolate agent subprocesses in their own process groupBased on @malpou's work in #27 (process group isolation and kill helper), with the addition of two-stage Ctrl+C to avoid corrupting work in progress.
Test plan
start_new_session=True)🤖 Generated with Claude Code